Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invidious: 2.20240825.2 -> 2.20241110.0 #355279

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Conversation

Kupac
Copy link
Contributor

@Kupac Kupac commented Nov 11, 2024

Due to some changes on Youtube, channels and subscriptions were no longer displayed in invidious. This update solves the problem.
iv-org/invidious#5021
iv-org/invidious#5059

Things done

Updated using the included update.sh script.

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@_999eagle, @GaetanLepage, @sbruder, @pbsds

@hughobrien
Copy link
Contributor

They mention a new dependency on tzdata, was that something we needed to include?

@hughobrien
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 355279


x86_64-linux

✅ 1 package built:
  • invidious

@pbsds
Copy link
Member

pbsds commented Nov 11, 2024

VM seems to run fine
image
(i assume my ip is blacklisted, since my instance in known by some)

They mention a new dependency on tzdata, was that something we needed to include?

Judging by this line

https://github.com/iv-org/invidious/blob/98926047586154269bb269d01e3e52e60e044035/docker/Dockerfile#L36

it seems it is needed at runtime, not build time. In my minimal VM i find

image

But it would be nice to encode this explicitly in the nixos module if possible

@pbsds pbsds mentioned this pull request Nov 11, 2024
13 tasks
@Kupac
Copy link
Contributor Author

Kupac commented Nov 12, 2024

Should I simply add it to buildInputs? I guess it worked on my system because tzdata must be installed already (which should be the case for most systems). But apparently alpine based docker is not one of them.

@hughobrien
Copy link
Contributor

hughobrien commented Nov 12, 2024

I honestly don't know, many tzdata usages in nixpkgs seem to also involve patching in zoneinfo paths.

According to the bug report it should be obvious enough in the logs if it turns out to be necessary. Requires navigating to the subscriptions page.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 12, 2024
@Kupac
Copy link
Contributor Author

Kupac commented Nov 12, 2024

So we can go ahead, as it will fix invidious on most systems. If tzdata problems appear, we can address it in a separate pr. I don't know a good way to reproduce the potential bug anyway.

@fpletz
Copy link
Member

fpletz commented Nov 12, 2024

Deployed on aarch64 and fixes the issues mentioned.

@fpletz fpletz merged commit 4361836 into NixOS:master Nov 12, 2024
32 of 33 checks passed
@Kupac Kupac deleted the invidious_update branch November 12, 2024 12:18
Copy link
Contributor

Successfully created backport PR for release-24.05:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants